Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: update execution-spec-tests to 1.0.5 #28337

Merged
merged 4 commits into from
Oct 16, 2023

Conversation

holiman
Copy link
Contributor

@holiman holiman commented Oct 13, 2023

This updates the execution-spec-tests to latest version. Putting it up to see if it builds

@holiman
Copy link
Contributor Author

holiman commented Oct 16, 2023

So the tests passes, yay ... however, we don't actually execute the cancun stuff

	bt.skipLoad(`^cancun/`)
	bt.skipLoad(`-fork=Cancun`)

So let's not merge this until we get that working

@holiman
Copy link
Contributor Author

holiman commented Oct 16, 2023

@marioevz to narrow the failures down a bit, I'm getting for example, on mcopy_huge_memory_expansion.json/000-fork=Cancun-from_existent_memory an error.
The expected stateroot for block 1 is 27557b93e5bb671d712b61c0da570f6883abbc7bd2da95f0af4e462f139f41bb, but I am getting 1051394ef6cb6fbcc2838024c0ca9ff6ca975ef4aefc041d82608ce36f44e7c7.

This happens:

  • The genesis matches (correct genesis stateroot etc).
  • (there's no beacon blockroot contract, so the system call to that addy doesn't affect anything)
  • Block 1 invokes an mcopy, with (dst 115792089237316195423570985008687907853269984665640564039457584007913129639935, src 0, length 1), which naturally triggers an error.
=== RUN   TestExecutionSpec/mcopy_huge_memory_expansion.json/000-fork=Cancun-from_existent_memory-successful=False--max_dest_single_byte_expansion
    block_test.go:77: test in hash mode without snapshotter failed: block #1 insertion into chain failed: invalid merkle root (remote: 27557b93e5bb671d712b61c0da570f6883abbc7bd2da95f0af4e462f139f41bb local: 1051394ef6cb6fbcc2838024c0ca9ff6ca975ef4aefc041d82608ce36f44e7c7) dberr: %
!w(<nil>)

@holiman
Copy link
Contributor Author

holiman commented Oct 16, 2023

[user@work go-ethereum]$ go run   ./cmd/evm/ --json  blocktest ./tests/spec-tests/fixtures/cancun/eip5656_mcopy/mcopy_memory_expansion/mcopy_huge_memory_expansion.json
...
{"pc":47,"op":94,"gas":"0x5dd9e6e","gasCost":"0x3","memSize":256,"stack":["0x0","0x0","0x1","0x0","0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff"],"depth":2,"refund":0,"opName":"MCOPY","error":"gas uint64 overflow"}
{"pc":38,"op":85,"gas":"0x17d5d2","gasCost":"0x5654","memSize":256,"stack":["0x1","0x0"],"depth":1,"refund":0,"opName":"SSTORE"}
{"pc":39,"op":0,"gas":"0x177f7e","gasCost":"0x0","memSize":256,"stack":[],"depth":1,"refund":0,"opName":"STOP"}
...
########## BAD BLOCK #########
Block: 1 (0x0133b010e20067d6f0c1fac34c8245b8968d87d968ca3096ef8eeeb14d7e92c0)
Error: invalid merkle root (remote: 27557b93e5bb671d712b61c0da570f6883abbc7bd2da95f0af4e462f139f41bb local: 1051394ef6cb6fbcc2838024c0ca9ff6ca975ef4aefc041d82608ce36f44e7c7) dberr: %!w(<nil>)

@holiman
Copy link
Contributor Author

holiman commented Oct 16, 2023

This is the (complete) state that I get

{
    "root": "1051394ef6cb6fbcc2838024c0ca9ff6ca975ef4aefc041d82608ce36f44e7c7",
    "accounts": {
        "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b": {
            "balance": "10779762",
            "nonce": 1,
            "root": "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421",
            "codeHash": "0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470"
        }
    }
}

Copy link
Member

@danceratopz danceratopz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @holiman, execution-spec-tests artifact naming changed in v1.0.5. The name of the fixtures file to download in order to get fixtures for features under development is now:

fixtures_develop.tar.gz

L337 needs to be changed accordingly here:

go-ethereum/build/ci.go

Lines 331 to 337 in 509a64f

func downloadSpecTestFixtures(csdb *build.ChecksumDB, cachedir string) string {
executionSpecTestsVersion, err := build.Version(csdb, "spec-tests")
if err != nil {
log.Fatal(err)
}
ext := ".tar.gz"
base := "fixtures" // TODO(MariusVanDerWijden) rename once the version becomes part of the filename

I'm not sure we'll ever include the version in the filename though as it only complicates downloading the artifacts. Is that important for you?

And the suggested change below is also required.

Sorry about that! More info in "breaking changes" in the release notes:
https://github.com/ethereum/execution-spec-tests/releases/tag/v1.0.5

# https://github.com/ethereum/execution-spec-tests/releases/download/v1.0.2/
24bac679f3a2d8240d8e08e7f6a70b70c2dabf673317d924cf1d1887b9fe1f81 fixtures.tar.gz
# https://github.com/ethereum/execution-spec-tests/releases/download/v1.0.5/
322085c2c2e173069d38d79034518e91bd53b29eaafd92a56f0ef753b5336aa7 fixtures.tar.gz
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
322085c2c2e173069d38d79034518e91bd53b29eaafd92a56f0ef753b5336aa7 fixtures.tar.gz
d4fd06a0e5f94beb970f3c68374b38ef9de82d4be77517d326bcf739c3cbf3a2 fixtures_develop.tar.gz

@holiman
Copy link
Contributor Author

holiman commented Oct 16, 2023

Thanks @danceratopz !

@holiman
Copy link
Contributor Author

holiman commented Oct 16, 2023

Now the failing ones are

  • cancun/eip4788_beacon_root/beacon_root_contract/beacon_root_precompile_calls.json (000-011),
  • cancun/eip4788_beacon_root/beacon_root_contract/beacon_root_precompile_timestamps.json (000-023) and
  • cancun/eip4788_beacon_root/blocks_beacon_root_contract/beacon_root_transition_test.json

@holiman
Copy link
Contributor Author

holiman commented Oct 16, 2023

The last one fails to import block 15, with

Block: 15 (0x15eabba755d11877f1e4da90d93c0805d33472521991b795a497093a43ec1caf)
Error: invalid gas used (remote: 166464 local: 124548)

One thing I noticed was that it calls 0xb:

{"pc":22,"op":241,"gas":"0xeef7f","gasCost":"0x190cb","memSize":32,"stack":["0x20","0x20","0x20","0x0","0x0","0xb","0x186a0"],"depth":1,"refund":0,"opName":"CALL"}

Is that maybe a remnant from when the beacon root was at 0xb?

@danceratopz
Copy link
Member

Now the failing ones are

* `cancun/eip4788_beacon_root/beacon_root_contract/beacon_root_precompile_calls.json` (`000`-`011`),

* `cancun/eip4788_beacon_root/beacon_root_contract/beacon_root_precompile_timestamps.json` (`000`-`023`) and

* `cancun/eip4788_beacon_root/blocks_beacon_root_contract/beacon_root_transition_test.json`

Is that maybe a remnant from when the beacon root was at 0xb?

@holiman, looking at the names and the fail, it seems to me that these are indeed leftover artifacts from v1.0.2 that refer to an out-date beacon root spec. Could it be that the folder that the CI downloads fixtures into isn't cleaned between runs?

@holiman
Copy link
Contributor Author

holiman commented Oct 16, 2023

@danceratopz aargh, right, again: thanks!

@holiman holiman marked this pull request as ready for review October 16, 2023 15:11
@holiman holiman merged commit 4632b7b into ethereum:master Oct 16, 2023
2 checks passed
@holiman holiman added this to the 1.13.4 milestone Oct 16, 2023
devopsbo3 pushed a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
Updates execution-spec-tests to 1.0.5: https://github.com/ethereum/execution-spec-tests/releases/tag/v1.0.5, switching to develop which contains Cancun tests (which are also enabled in this change).
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants